-
Notifications
You must be signed in to change notification settings - Fork 118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
objcachev2 #522
objcachev2 #522
Conversation
common/objectcachev2.h
Outdated
// other reader will get new one after updated | ||
std::shared_ptr<V> update(V* val, uint64_t ts = 0, | ||
std::shared_ptr<V>* newptr = nullptr) { | ||
auto r = std::shared_ptr<V>(val); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you manage to integrate shared_ptr into ctor, so that there's a chance to use make_shared to improve performance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here accept plain V*
type pointer is for compatibility, the ctor
in ObjectCache
returns only simple plain pointer.
It seems good to have a new specialization accept ctor
returns shared_ptr directly.
common/objectcachev2.h
Outdated
LockfreeMPMCRingQueue<std::shared_ptr<V>*, 4096>> | ||
reclaim_queue; | ||
photon::spinlock maplock; | ||
std::unordered_set<Box*, ItemHash, ItemEqual> map; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to use unordered_set<Box, ItemHash, ItemEqual>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is possible, the code might be a little tricky but tested work.
common/objectcachev2.h
Outdated
|
||
std::shared_ptr<V> __update(Box* item, V* val) { | ||
std::shared_ptr<V> ret; | ||
reclaim_queue.template send<PhotonPause>(new std::shared_ptr<V>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
split it into two statements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new std::shared_ptr<V>(...)
looks evil. Try to avoid it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lockfree queue accepts only POD types, a shared_ptr<T>
object cannot sen via lockfree queue.
Maybe I can try to specialize a lockfree queue with shared_ptr<T>
type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
~ObjectCacheV2() { | ||
_timer.stop(); | ||
if (reclaimer) { | ||
reclaim_queue.template send<PhotonPause>(nullptr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it necessary to use the template
keyword?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is necessary.
Calling template member function needs a template
prefix. The send<P>
member function has default template argument ThreadPause
, but here has to call with argument PhotonPause
, makes here have to be with a template
prefix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have following code in RPC's exmaple:
ExampleServer()
: skeleton(photon::rpc::new_skeleton()),
server(photon::net::new_tcp_socket_server()) {
skeleton->register_service<Testrun, Heartbeat, Echo, ReadBuffer,
WriteBuffer>(this);
}
We call the template member function, register_service<...>
without a template
keyword.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah but those code in RPC is a part of normal class.
Here is in template define.
ignore that template
prefix will leads to erros
error: expected primary-expression before ‘>’ token
in GCC and
Use 'template' keyword to treat 'send' as a dependent template name (fix available)clang(missing_dependent_template_keyword)
in clang
rpc/test/test.cpp
Outdated
@@ -481,7 +481,8 @@ TEST_F(RpcTest, passive_shutdown) { | |||
|
|||
// The passive shutdown took 3 seconds, until client closed the connection | |||
GTEST_ASSERT_GT(duration, 2900); | |||
GTEST_ASSERT_LT(duration, 3200); | |||
// Since GH macos arm CI is slow, we allow 3.5 secs | |||
GTEST_ASSERT_LT(duration, 3500); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm moving it to release/0.7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
Signed-off-by: Coldwings <[email protected]>
Signed-off-by: Coldwings <[email protected]>
Signed-off-by: Coldwings <[email protected]>
A simpler but effective object cache implementation.
ObjectCacheV2
shows better performance compare toObjectCache
, with severalimprovements:
update
API, able to immediately substitute objects.ObjectCacheV2
no longer support acquire/release API.It should work as
ObjectCache
in code do not depends on acquire/release API.